Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Issue-840 #841

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

WIP: Issue-840 #841

wants to merge 2 commits into from

Conversation

jbcooley7
Copy link

WIP: Testing tox.ini and requirements text file changes since I don't have all the Python versions installed in order to get tox running locally.

Copy link
Contributor

@jay-tyler jay-tyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing failing is a CI check for black. Running black -l 100 hug will be enough to get the CI to pass. And then there are a few blank lines in the requirements files, if you wouldn't mind deleting these. Would be happy to merge then

numpy==1.15.4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for the reordering and blank lines? Just curious

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly as a visual aid as I was comparing requirements across files. I do like to have requirements having a similar purpose grouped together though, like the pytest requirements. I can remove the blank lines, or completely revert the file if you'd prefer.

@@ -1,16 +1,16 @@
bumpversion==0.5.3
Cython==0.29.10

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the reordering and blank lines in this file?

@@ -4,24 +4,33 @@ envlist=py{35,36,37,38,py3}-marshmallow{2,3}, cython-marshmallow{2,3}
[testenv]
deps=
-rrequirements/build_common.txt
marshmallow2: marshmallow <3.0
marshmallow3: marshmallow==3.0.0rc6
marshmallow2: -rrequirements/marshmallow2.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate the cleanup in this file. Really want your PR merged for this.

@jbcooley7
Copy link
Author

As far as I can tell, the "testdevelopment" section I added to the tox.ini file is not being run, so I'll need to figure out why before these changes satisfy the requirements in the ticket.

@jay-tyler
Copy link
Contributor

@jbcooley7 Does this help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants